-
-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use XLSForm injection during project creation #1792
Conversation
@Sujanadh sorry to distract from the fmtm-splitter work, but could you please take a look at this PR tomorrow? I was coding into the night, so no doubt have missed something. Please check everything works as intended / looks good to you π This will finalise the work on injecting XLSForm fields and allow #1653 to be easily solved. Thank you! |
sorry i didn't see the PR , i will test it |
Encountered few issues while testing will post it here briefly. |
Thanks! There are definitely some bugs to find π I didn't actually get to test updating an existing form either, so that probably needs some work! |
I used default form here. |
when trying to check custom form also there is issue
this was the particular reason that i had to first append mandatory fields before testing for validation. oh actually my bad , i should debug on for custom form , it works fine on custom form |
i don't think we need to validate it again in |
Thanks for the tests! The issue about Agree we could validate once only! While it's quite likely our frontend is the only user of the API, calling validate-form prior to generating project files, we can't guarantee it. So validating again is just an addition check, without much performance penalty - what do you think? |
Regarding the default forms, I already updated the forms to remove the verification fields (so they are injected). I will update them again now to remove the FMTM mandatory fields entirely, and have them injected the same way as a custom form. This should improve maintainability, as we don't need to update the entire XLSForm library when we change something in the mandatory fields π Related issue I just made: hotosm/osm-fieldwork#300 |
This is a valid error though! We are having issues with the dataframe merging logic |
Ah got it, you are right that makes sense. |
Made some updates! Hopefully fixed project creation and XLSForm updating π |
I made some updates to osm-fieldwork for this, so the main things that need to be done:
I am merging this now so we can more easily test - we can make another PR if needed. |
What type of PR is this? (check all applicable)
Related Issue
Fixes #1722
Describe this PR
validate-form
endpoint.Review Guide
Checklist before requesting a review
[optional] What gif best describes this PR or how it makes you feel?